Thanks for this @oswin-mako — really comprehensive coverage across IAM, networking, storage, encryption, logging, compute, and Cloud Run. To validate it end-to-end, I provisioned a real GCP test environment (Terraform module with BAD/COMPLIANT resource pairs across all 7 domains) and ran the scanner against it. Surfaced a few issues worth fixing before merge.

I've pushed a reference branch with the rebase + four follow-up commits at https://github.com/transilienceai/shasta/tree/review/pr-20-gcp-fixes — feel free to cherry-pick the commits you agree with, or write your own fix and we can drop the reference branch.

Rebase needed

The branch is currently DIRTY against main (PR #21 / voice-console landed and touched README.md, TRUST.md, CHANGELOG.md). Resolution preserves both your numeric bumps and the voice-console wording. Also: rebasing onto current main makes tests/test_integrity/test_doc_claims.py::test_root_trust_test_count fail because the doc says 720 tests and the rebased tree collects 839. Both fixes are in commit 8c22945 on the reference branch.

Bug 1 — KMS location wildcard returns 404

src/shasta/gcp/encryption.py:56 calls

kms.projects().locations().keyRings().list(parent=f"projects/{project_id}/locations/-")

GCP rejects - as a wildcard for KMS location listing — every project produces a single NOT_ASSESSED finding (HttpError 404: "The request concerns location '-' but was..."), masking all KMS keys regardless of rotation state. Verified against a project with 3 KMS keys (2 non-compliant, 1 compliant) — pre-fix scanner output:

[gcp-kms-key-rotation] Unable to list KMS key rings
  description: API call failed: <HttpError 404 ... locations/- ...>

Fix in commit afd65cf: enumerate KMS-supported locations first via kms.projects().locations().list(name="projects/X"), then list keyRings per location. Per-location failures don't abort the whole check. Same project after the fix correctly emits 1 FAIL with the 2 offenders enumerated.

KMS unit test mocks updated for the new API call shape — the chain setup needs an extra mock entry:

kms.projects.return_value.locations.return_value.list.return_value.execute.return_value = {
    "locations": [{"locationId": "global"}]
}

Bug 2 — Subnet checks emit per-region aggregates, not per-subnet

check_subnet_flow_logs_enabled and check_private_google_access_enabled (src/shasta/gcp/networking.py:374 and :460) collect non-compliant subnet names into a missing list, then emit one aggregate finding per region with all the offending names jammed into the description string. Side effects:

1. Compliant subnets in regions that also have non-compliant subnets get no PASS finding — they're invisible.
2. The aggregate finding's resource_id is projects/X/regions/Y/subnetworks (a query path, not a real resource) — risk register entries and remediation tracking can't link to the actual offender.
3. The auto-created default VPC populates a subnet in every GCP region, so a clean test project produces ~40 region-aggregate FAILs that hide the COMPLIANT subnet a user explicitly created.

Fix in commit e7eea3c: emit one finding per subnet, with resource_id derived from the subnet's selfLink. Pre-fix output had 42 FAIL × 0 PASS for our test env; post-fix has 43 FAIL × 1 PASS (the COMPLIANT subnet now visible).

Bug 3 — Token-creator check ignores SA holders at project scope

check_iam_service_account_token_creator at src/shasta/gcp/iam.py:510:

non_sa = [m for m in members if not m.startswith("serviceAccount:")]
if non_sa:
    offenders.append({"role": role, "members": non_sa})

The filter excludes service-account holders from offenders. So a project-level binding of roles/iam.serviceAccountTokenCreator granted to a service account silently passes — even though compromise of that SA produces the same blast radius as a human holder (impersonation of every SA in the project). Verified: granted the role to shasta-bad-tokencreator-sa@... at project scope → gcloud projects get-iam-policy confirms binding exists → check returned PASS.

This is technically defensible as a strict CIS GCP 1.6 reading ("IAM Users... at Project Level"), but:

• The check's own docstring says the role "should exist only in tightly scoped resource-level bindings, not at the project level" — full stop, no carve-out for SA holders.
• Industry tools (Prowler, ScoutSuite, CloudSploit) flag any holder type at project scope.
• Google's least-privilege docs recommend resource-level scoping regardless of principal type.

Fix in commit 79076fc: drop the non_sa filter, flag any project-level binding. Updated docstring + finding text to make the broad scope explicit. If the strict CIS reading is preferred, that's a reasonable counter-argument — happy to flip it back, but I'd suggest also adding a separate check_iam_token_creator_users_only so the granular interpretation isn't lost.

Things working well in PR #20

• _run_gcp_checks failure handling (scanner.py:451-469) — wrapping each runner in try/except + emitting gcp-<domain>-scan-error NOT_ASSESSED is exactly the right pattern (engineering principle #6: "treat empty results different from errors"). Caught a real case in testing where Cloud Run me-central2 returns 403 — surfaced cleanly without crashing the scan.
• cis_gcp_controls field on Finding model — populated consistently across all the GCP modules, matches the AWS/Azure pattern.
• 130 unit tests covering check logic + 17 Terraform template renders. Solid test density.

Coverage gap (separate from the bugs above)

Three checks have no unit-test coverage at all — check_iam_no_allusers_access, check_iam_service_account_token_creator, check_iam_workload_identity_preferred. Bug #3 above survived because nothing tested it. Worth a follow-up commit before merge.

Validation summary

130/130 GCP unit tests green after all three fixes (one mock update needed for KMS). Live scan against the test project produces 125 findings across 34 distinct check IDs — BAD fixtures FAIL, COMPLIANT fixtures PASS, error paths emit NOT_ASSESSED instead of misleading PASS results.

Happy to discuss any of the fixes or open separate issues for the coverage gap. Thanks again for the contribution.